Skip to content

Fix newly introduced python warnings in XenAPI.py #5761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jul 2, 2024
Merged

Fix newly introduced python warnings in XenAPI.py #5761

merged 2 commits into from
Jul 2, 2024
+4 −3

Conversation

edwintorok
Copy link
Contributor

Apparently some more checks got enabled on master for python that fail on the changes done to XenAPI.py on the feature branch (but these tests didn't fail at the time the changes were made, they probably weren't active yet).

edwintorok added 2 commits July 1, 2024 16:34
@edwintorok
fix(XenAPI.py): fix pylint warning
c460360 
This code snippet was unchanged since the beginning,
but pylint complained:
```
R1720: Unnecessary "else" after "raise", remove the "else" and de-indent the code inside it (no-else-raise)
```

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
@edwintorok
fix(XenAPI.py): fix pylint warning
84c2ba6 
Fixes: 500a1f7 ("XenAPI: suppress pytype false positives")

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
psafont
psafont reviewed Jul 1, 2024
View reviewed changes
@edwintorok
Copy link
Contributor Author

edwintorok commented Jul 1, 2024 •
edited
Loading

Hmm the new parallel finish fails:

/home/runner/work/_actions/coverallsapp/github-action/v1/dist/index.js:40831
                        throw new Error('Parallel webhook error:' + err + ', ' + JSON.stringify(data));
                        ^

Error: Parallel webhook error:Error: {"error":"No build matching CI build number 9747[17](https://github.com/xapi-project/xen-api/actions/runs/9747174321/job/26899347309?pr=5761#step:2:18)4321 found"}, {"error":"No build matching CI build number 9747174321 found"}
    at Request._callback (/home/runner/work/_actions/coverallsapp/github-action/v1/dist/index.js:40831:31)
    at Request.self.callback (/home/runner/work/_actions/coverallsapp/github-action/v1/dist/index.js:24734:22)
    at Request.emit (node:events:513:28)
    at Request.<anonymous> (/home/runner/work/_actions/coverallsapp/github-action/v1/dist/index.js:25703:10)
    at Request.emit (node:events:513:28)
    at IncomingMessage.<anonymous> (/home/runner/work/_actions/coverallsapp/github-action/v1/dist/index.js:25625:12)
    at Object.onceWrapper (node:events:627:28)
    at IncomingMessage.emit (node:events:525:35)
    at endReadableNT (node:internal/streams/readable:1358:12)
    at processTicksAndRejections (node:internal/process/task_queues:83:[21](https://github.com/xapi-project/xen-api/actions/runs/9747174321/job/26899347309?pr=5761#step:2:22))

I'll remove it from this PR for now.

@edwintorok edwintorok mentioned this pull request Jul 1, 2024
Merged
@bernhardkaindl bernhardkaindl mentioned this pull request Jul 1, 2024
Merged
bernhardkaindl
bernhardkaindl approved these changes Jul 1, 2024
View reviewed changes
Copy link
Collaborator

@bernhardkaindl bernhardkaindl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explanation: The line to just disable the warning on self._session needed to be converted to use the block syntax for disabling, because the line was too long with the disable comment at the end of the line.

@edwintorok edwintorok marked this pull request as ready for review July 2, 2024 08:31
@edwintorok edwintorok mentioned this pull request Jul 2, 2024
Merged
@edwintorok edwintorok mentioned this pull request Jul 2, 2024
Merged
@psafont psafont merged commit c060abf into xapi-project:feature/perf Jul 2, 2024
29 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@psafont psafont psafont approved these changes

@bernhardkaindl bernhardkaindl bernhardkaindl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@edwintorok @psafont @bernhardkaindl